Skip to content

chore: upgrade lerna #2836

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 75 commits into from
May 22, 2025
Merged

Conversation

david-luna
Copy link
Contributor

@david-luna david-luna commented May 19, 2025

Which problem is this PR solving?

This is a follow up PR of #2493. It updates lerna to version v8.2.2, which indirectly updates nx. Dev scripts are using nx directly but publish workflows and tasks keep using lerna

Changes

  • remove unnecessary scripts in packages (precompile & prewatch)
  • upgrade lerna dependency
  • update scripts/peer-api-check.mjs to work at the root level and the dependant workflows
  • update scripts/check-release-please.mjs, simplification
  • added a setup:dev npm script in each package for people who does not want to compile all project to start developing
  • update docs

Supersedes: #2526

@david-luna david-luna marked this pull request as ready for review May 20, 2025 12:30
Copy link
Member

@pichlermarc pichlermarc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks good % a few nits, thanks for working on this 🙂

I think RELEASING.md can be removed as we likely won't release manually anymore now that we also add provenance statements to our published packages.

popd
done
```sh
npx lerna publish from-package --no-push --no-private --no-git-tag-version --no-verify-access --yes
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the whole RELEASING.md can be removed, or we can refer to the release-please workflow that basically handles all the releases now. 🙂

(In the 2.5 years I've been a maintainer, I've never needed to do a manual release in the contrib repo 🙂)

"watch": "tsc --build --watch tsconfig.json tsconfig.esm.json tsconfig.esnext.json",
"precompile": "lerna run version --scope @opentelemetry/propagator-aws-xray --include-dependencies",
"prewatch": "npm run precompile"
"watch": "tsc --build --watch tsconfig.json tsconfig.esm.json tsconfig.esnext.json"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Q: I am ignorant here. Does removing this precompile mean that I see the npm run compile at the top-level before working on this package? (Maybe that is already a requirement.)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(Maybe that is already a requirement)

That's what I interpreted from the contributing guide

CONTRIBUTING.md Outdated
@@ -119,6 +119,29 @@ Some tests depend on other packages to be installed, so these steps are also req

Each of these commands can also be run in individual packages, as long as the initial install and compile are done first in the root directory.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it woudl be helpful to have a separate section header for this new focus thing.

popd
done
```sh
npx lerna publish from-package --no-push --no-private --no-git-tag-version --no-verify-access --yes
Copy link
Contributor

@trentm trentm May 21, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From the top of this file:

For posterity, or in the event of any failures with release-please, the process for performing a manual release is below.

Given this gets out of date, it would possibly be better to drop it. I'd defer to @pichlermarc for an opinion on that as he does most of the releasing.

Update: Marc already gave an opinion on this above (#2836 (comment)).

CONTRIBUTING.md Outdated
> NX Successfully ran target compile for project @opentelemetry/resource-detector-aws and 3 tasks it depends on (7s)
```

Once the command is done you can `cd` into the package and start using the usual commands.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(non-blocking) I'm not opposed to this command, but it would be kind of nice to not have to type out both (a) the package name and (b) the directory for that package.

Possible alternative:

cd detectors/node/opentelemetry-resource-detector-aws   # or whatever package to focus on
npm run compile:include-deps

Or perhaps others would find a command at the top-level more convenient? My personal typical development process is that I cd into the package dir I am working on and work from there. So an npm run ... script in that package dir would be convenient for me.

Copy link
Contributor Author

@david-luna david-luna May 22, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since I was already removing scripts from each package.json I didn't want to add anything else. But I think its a fair point. At the end you have to cd into that directory to run the scripts there so it would be more convenient.

Checklist:

  • remove focus script
  • add setup:dev script in each package
  • update contributing.md: remove focus section
  • update contributing.md: rewrite compile at the top requirement

david-luna and others added 5 commits May 21, 2025 20:39
@david-luna
Copy link
Contributor Author

There is a test failing in @opentelemetry/instrumentation-undici when doing TAV. I'll check this and make another PR if necessary.

-- required packages ["undici@6.21.3"]
-- installing ["undici@6.21.3"]
-- running test "npm run test" with undici (env: {})

> @opentelemetry/instrumentation-undici@0.12.0 test
> nyc mocha test/**/*.test.ts



  UndiciInstrumentation `fetch` tests
    disable()
      ✔ should not create spans when disabled (40ms)
    enable()
      ✔ should create valid spans even if the configuration hooks fail
      ✔ should create valid spans with empty configuration
      ✔ should create valid spans with the given configuration
      ✔ should not create spans without parent if required in configuration
      ✔ should not create spans with parent if required in configuration
      1) should capture errors using fetch API
      ✔ should capture error if fetch request is aborted

  UndiciInstrumentation metrics tests
    with fetch API
      ✔ should report "http.client.request.duration" metric
      ✔ should have error.type in "http.client.request.duration" metric

  UndiciInstrumentation `undici` tests
    disable()
      ✔ should not create spans when disabled
    enable()
      ✔ should ignore requests based on the result of ignoreRequestHook
      ✔ should create valid spans for different request methods
      ✔ should create valid spans for "request" method
      ✔ should create valid spans for "fetch" method
      ✔ should create valid spans for "stream" method
      ✔ should create valid spans for "dispatch" method
      ✔ should create valid spans even if the configuration hooks fail
      ✔ should not create spans without parent if required in configuration
      ✔ should not create spans with INVALID_SPAN_CONTEXT parent if required in configuration
      ✔ should create spans with parent if required in configuration
      2) should capture errors while doing request
      ✔ should capture error if undici request is aborted
      ✔ should not report an user-agent if it was not defined
      3) should create valid span if request.path is a full URL


  22 passing (5s)
  3 failing

  1) UndiciInstrumentation `fetch` tests
       enable()
         should capture errors using fetch API:
     Error: Timeout of 2000ms exceeded. For async tests and hooks, ensure "done()" is called; if returning a Promise, ensure it resolves. (/home/runner/work/opentelemetry-js-contrib/opentelemetry-js-contrib/plugins/node/instrumentation-undici/test/fetch.test.ts)
      at listOnTimeout (node:internal/timers:581:17)
      at processTimers (node:internal/timers:519:7)

  2) UndiciInstrumentation `undici` tests
       enable()
         should capture errors while doing request:
     Error: Timeout of 2000ms exceeded. For async tests and hooks, ensure "done()" is called; if returning a Promise, ensure it resolves. (/home/runner/work/opentelemetry-js-contrib/opentelemetry-js-contrib/plugins/node/instrumentation-undici/test/undici.test.ts)
      at listOnTimeout (node:internal/timers:581:17)
      at processTimers (node:internal/timers:519:7)

  3) UndiciInstrumentation `undici` tests
       enable()
         should create valid span if request.path is a full URL:

      AssertionError [ERR_ASSERTION]: Expected values to be strictly equal:

2 !== 1

      + expected - actual

      -2
      +1
      
      at Context.<anonymous> (test/undici.test.ts:852:14)
      at processTicksAndRejections (node:internal/process/task_queues:95:5)

Co-authored-by: Trent Mick <trentm@gmail.com>
@david-luna david-luna enabled auto-merge (squash) May 22, 2025 18:28
@david-luna david-luna merged commit f04c640 into open-telemetry:main May 22, 2025
22 of 23 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pkg:auto-configuration-propagators pkg:auto-instrumentations-node pkg:auto-instrumentations-web pkg:host-metrics pkg:id-generator-aws-xray pkg:instrumentation-amqplib pkg:instrumentation-aws-lambda pkg:instrumentation-aws-sdk pkg:instrumentation-bunyan pkg:instrumentation-cassandra-driver pkg:instrumentation-connect pkg:instrumentation-cucumber pkg:instrumentation-dataloader pkg:instrumentation-dns pkg:instrumentation-document-load pkg:instrumentation-express pkg:instrumentation-fastify pkg:instrumentation-fs pkg:instrumentation-generic-pool pkg:instrumentation-graphql pkg:instrumentation-hapi pkg:instrumentation-ioredis pkg:instrumentation-kafkajs pkg:instrumentation-knex pkg:instrumentation-koa pkg:instrumentation-long-task pkg:instrumentation-lru-memoizer pkg:instrumentation-memcached pkg:instrumentation-mongodb pkg:instrumentation-mongoose pkg:instrumentation-mysql pkg:instrumentation-mysql2 pkg:instrumentation-nestjs-core pkg:instrumentation-net pkg:instrumentation-oracledb pkg:instrumentation-pg pkg:instrumentation-pino pkg:instrumentation-redis pkg:instrumentation-redis-4 pkg:instrumentation-restify pkg:instrumentation-router pkg:instrumentation-runtime-node pkg:instrumentation-socket.io pkg:instrumentation-tedious pkg:instrumentation-undici pkg:instrumentation-user-interaction pkg:instrumentation-winston pkg:plugin-react-load pkg:propagation-utils pkg:propagator-aws-xray pkg:propagator-aws-xray-lambda pkg:propagator-instana pkg:propagator-ot-trace pkg:redis-common pkg:resource-detector-alibaba-cloud pkg:resource-detector-aws pkg:resource-detector-azure pkg:resource-detector-container pkg:resource-detector-gcp pkg:resource-detector-github pkg:resource-detector-instana pkg:sampler-aws-xray pkg:sql-common pkg:test-utils pkg-status:unmaintained:autoclose-scheduled pkg-status:unmaintained This package is unmaintained. Only bugfixes may be acceped until a new owner has been found.
Projects
None yet
Development

Successfully merging this pull request may close these issues.